Scripts: Security enablement during west build #87669

Aasimshaik11 · 2025-03-26T09:45:22Z

Added changes to automate the signing of zephyr security using customtool during the west build process.

github-actions · 2025-03-26T09:45:54Z

Hello @Aasimshaik11, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

asmellby · 2025-04-04T09:51:46Z

CMakeLists.txt

Please revert these unrelated changes.

Resolved this comment

asmellby · 2025-04-04T09:52:04Z

boards/silabs/radio_boards/siwx917_rb4338a/Kconfig.siwx917_rb4338a

@@ -3,3 +3,25 @@

 config BOARD_SIWX917_RB4338A
 	select SOC_PART_NUMBER_SIWG917M111MGTBA
+
+# Kconfig.board for SiWx917 RB4338A


Unnecessary comment, please remove.

Resolved this comment

asmellby · 2025-04-04T09:53:34Z

boards/silabs/radio_boards/siwx917_rb4338a/Kconfig.siwx917_rb4338a

+
+# Kconfig.board for SiWx917 RB4338A
+
+config SIGNING_ENABLED


Signing support should be a property of the SiWx91x SoC, not of the board. All of this must move to soc/silabs/silabs_siwx91x/, and the Kconfig options must be renamed to show that they apply to 91x only.

Resolved this comment

asmellby · 2025-04-04T09:53:58Z

boards/silabs/radio_boards/siwx917_rb4338a/board.cmake

@@ -11,3 +11,71 @@ include(${ZEPHYR_BASE}/boards/common/silabs_commander.board.cmake)
 # Once started, it should be possible to reset the device with "monitor reset"
 board_runner_args(jlink "--device=Si917" "--speed=10000")
 include(${ZEPHYR_BASE}/boards/common/jlink.board.cmake)
+
+# Custom signing with Silicon Labs Commander


Move to SoC level

Resolved this comment

asmellby · 2025-04-04T09:54:36Z

boards/silabs/radio_boards/siwx917_rb4338a/board.cmake

+
+  # Define paths and keys from Kconfig
+  set(WORKSPACE_ROOT $ENV{PWD})
+  set(SL_COMMANDER_PATH $ENV{PWD}/SimplicityCommander-Linux/commander/commander)


You can't hardcode the path to Commander. You must assume that it exists on PATH.

Ok I resolved this

Resolved this comment

asmellby · 2025-04-04T09:55:48Z

scripts/west_commands/sign.py

@@ -79,6 +80,21 @@
 [rimage] sections in your west config file(s); this is especially useful
 when invoking west sign _indirectly_ through CMake/ninja. See how at
 https://docs.zephyrproject.org/latest/develop/west/sign.html
+
+sirps


This should be silabs_rps or silabs_commander_rps for consistency with the silabs_commander runner.

Resolved this comment

asmellby · 2025-04-04T09:57:01Z

scripts/west_commands/sign.py

+
+        # Primary source: Board Kconfig defaults
+        board_dir = cache.get('BOARD_DIR') # Fallback to PWD if BOARD_DIR missing
+        board_kconfig = pathlib.Path(board_dir) / 'Kconfig.siwx917_r4338a'


You can't hard-code the board Kconfig path, this must generically work for all SiWx91x based boards.

Resolved this comment

asmellby · 2025-04-04T09:57:30Z

boards/silabs/radio_boards/siwx917_rb4338a/board.cmake

+    message(STATUS "Private key found at '${M4_PRIVATE_KEY}'")
+  endif()
+
+  # Define the commander signing command


Why is this calling Commander directly, and not invoking west sign?

The direct call to Silicon Labs Commander in board.cmake is intentional to integrate signing into the west build process, ensuring that zephyr.bin.rps is generated automatically as part of the build output for SiWx917-based boards. This approach is used to meet hardware requirements where a signed binary is mandatory for flashing or execution. Invoking west sign would require a separate post-build step, which we avoid here to streamline the workflow and guarantee that the signed artifact is available immediately after west build, consistent with the board’s firmware deployment needs.

Why would a post-build step need to be separately invoked just because it's calling west instead of Commander? There is no difference if west sign or commander is called as a custom command.

Yeah, that's right. We can do that too, But there will be extra call of west sign that's it.

If west sign used:
Cmakefile -> west sign -> sign.py -> commander for signing

If not west sign:
Cmake -> Commander

IMO it would be better to call west sign, in order to avoid duplicating the implementation. The overhead of an extra call is well worth avoiding two implementations of the same logic.

Resolved this comments

scripts/west_commands/sign.py

soc/silabs/silabs_siwx91x/Kconfig.soc

soc/silabs/silabs_siwx91x/CMakeLists.txt

scripts/west_commands/sign.py

pdgendt · 2025-04-09T09:47:38Z

Please, do not resolve comments as per the contribution guidelines.

soc/silabs/silabs_siwx91x/Kconfig.soc

jerome-pouiller · 2025-04-09T18:42:34Z

soc/silabs/silabs_siwx91x/Kconfig.soc

+  default "Give_full_path_to_SimplicityCommander-Linux/commander/commander"
+  depends on SILABS_COMMANDER_SIGN
+  help
+    Path to the Silicon Labs Commander executable, relative to the current working directory (e.g., $ENV{PWD}), used for signing the firmware binary.


Kconfig files are indented with one tab. The help message is indented with one tab + 2 spaces.

The help text word-warped at 80 columns.

Resolved this comment

jerome-pouiller · 2025-04-09T18:49:05Z

scripts/west_commands/sign.py

@@ -633,3 +648,144 @@ def sign(self, command, build_dir, build_conf, formats):

        os.remove(out_bin)
        os.rename(out_tmp, out_bin)
+
+class Silabs_rpsSigner(Signer):


Silabs_rpsSigner?

SilabsRPSSigner is this fine?

Resolved this comment

jerome-pouiller · 2025-04-09T18:52:46Z

scripts/west_commands/sign.py

+                        elif current_config == 'M4_PRIVATE_KEY':
+                            m4_private_key = value
+                        elif current_config == 'COMMANDER_PATH':
+                            commander_path = value


6 level of indentation is a lot, especially in a function of 125 lines. Probably you could introduce a subfunction.

Resolved this comment

jerome-pouiller · 2025-04-09T18:55:32Z

scripts/west_commands/sign.py

+            workspace_base = pathlib.Path(os.getcwd())
+            if not args.quiet:
+                command.inf(f"ZEPHYR_WORKSPACE_ROOT not set, using current directory: {workspace_base}")
+            soc_dir = 'GIVE PATH FOR KCONFIG.SOC AFTER WORKSPACE'  # Same relative path


Is it temporary code?

Resolved this comment

jerome-pouiller · 2025-04-09T18:56:22Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

+if(CONFIG_SILABS_COMMANDER_SIGN)
+
+    # Define input and output files
+    set(ZEPHYR_BINARY ${CMAKE_BINARY_DIR}/zephyr/zephyr.bin)


WHat is the difference with ${KERNEL_BIN_NAME}?

It takes the Kernel name which is already defined.

Please, let the reviewer resolve the comments. I think this is a valid point, instead of hard-coding the path, use the existing CMake variable.

Resolved this comment

soc/silabs/silabs_siwx91x/CMakeLists.txt

jerome-pouiller · 2025-04-09T18:59:40Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

+        COMMENT "Ensuring signing with Silicon Labs Commander is complete"
+    )
+else()
+    message(STATUS "CONFIG_SIGNING_ENABLED is not enabled")


I believe we have to manage the non-signed binaries.

If signed_binary is needed for the user it do's that by checking the variable CONFIG_SIGNING_ENABLED.
If not it goes through with the default one.

Resolved this comment

jerome-pouiller · 2025-04-09T19:00:30Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

+    set(SIGNED_RPS ${CMAKE_BINARY_DIR}/zephyr/zephyr.bin.rps)
+
+    # Use $ENV{PWD} as working directory
+    set(WORKSPACE_ROOT ${CMAKE_CURRENT_SOURCE_DIR})


What is the purpose of creating a variable with the exact same value of an existing variable?

Resolved this comment

jerome-pouiller · 2025-04-09T19:02:11Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

+    set(WORKSPACE_ROOT ${CMAKE_CURRENT_SOURCE_DIR})
+
+    # Define the west sign command with explicit working directory and environment
+    set(west_sign_cmd


All the variables are uppercase in CMakeLists.txt

Resolved this comment

jerome-pouiller · 2025-04-09T19:03:47Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

+
+    # Define input and output files
+    set(ZEPHYR_BINARY ${CMAKE_BINARY_DIR}/zephyr/zephyr.bin)
+    set(SIGNED_RPS ${CMAKE_BINARY_DIR}/zephyr/zephyr.bin.rps)


Be consistent in the variable names.

Resolved this comment

jerome-pouiller · 2025-04-09T19:05:02Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

+        ${WEST} sign
+        -t silabs_rps
+        -d ${CMAKE_BINARY_DIR}
+        --sbin ${SIGNED_RPS}


You have to handle `CONFIG_FLASH_LOAD_OFFSET``

Resolved this comment

jerome-pouiller

Could you split your commit in two parts:

Add support for west sign -r silabs_commander
Add support for west build -t signed-rps

jerome-pouiller · 2025-04-18T11:25:12Z

scripts/west_commands/sign.py

@@ -112,7 +125,7 @@ def do_add_parser(self, parser_adder):

        # general options
        group = parser.add_argument_group('tool control options')
-        group.add_argument('-t', '--tool', choices=['imgtool', 'rimage'],
+        group.add_argument('-t', '--tool', choices=['imgtool', 'rimage', 'silabs_rps'],


The name of the binary the user install is commander. This tool is referenced with silabs_commander in Zephyr. I suggest to not introduce a new naming scheme.

Resolved this comment

jerome-pouiller · 2025-04-18T11:27:01Z

scripts/west_commands/sign.py

@@ -633,3 +648,147 @@ def sign(self, command, build_dir, build_conf, formats):

        os.remove(out_bin)
        os.rename(out_tmp, out_bin)
+
+class SilabsRPSSigner(Signer):
+    """Signer class for Silicon Labs Commander tool via west sign -t silabs_rps."""


This file tends to use usual comments (starting with #) rather than docstring. Let's be consistent.

Resolved this comment

jerome-pouiller · 2025-04-18T11:30:14Z

scripts/west_commands/sign.py

+                            commander_path = value
+        else:
+            if not args.quiet:
+                command.inf(f"No Kconfig.defconfig found at {soc_kconfig}. Using default values.")


Why don't you retrieve the values from build_conf?

Resolved this comment

jerome-pouiller · 2025-04-18T11:34:56Z

scripts/west_commands/sign.py

+        if not m4_private_key:
+            command.die("M4_PRIVATE_KEY not set in Kconfig.defconfig and no --m4-private-key provided.")
+        if not commander_path:
+            command.die("COMMANDER_PATH not set in Kconfig.defconfig and no --commander-path provided.")


get_security_config() never return unset values. So, can any of these condition happen?

Resolved this comment

jerome-pouiller · 2025-04-18T11:37:17Z

scripts/west_commands/sign.py

+        if not m4_private_key:
+            command.die("M4_PRIVATE_KEY not set in Kconfig.defconfig and no --m4-private-key provided.")
+        if not commander_path:
+            command.die("COMMANDER_PATH not set in Kconfig.defconfig and no --commander-path provided.")


BTW, why do you enforce the user to provide the name of the commander command. It is very likely commander.

Resolved this comment

jerome-pouiller · 2025-04-18T12:09:23Z

soc/silabs/silabs_siwx91x/Kconfig.defconfig

+	default "Full_path_to_m4_private_key.pem"
+
+config COMMANDER_PATH
+	default "Full_path_to_SimplicityCommander-Linux/commander/commander"


Nope, the usual user will have commander in hs PATH.

Resolved this comment

jerome-pouiller · 2025-04-18T12:09:48Z

soc/silabs/silabs_siwx91x/Kconfig.defconfig

+	default "Full_OTA_KEY_in_HEX"
+
+config M4_PRIVATE_KEY
+	default "Full_path_to_m4_private_key.pem"


Why do you provide default values if they are not correct?

Modified the code

Resolved this comment

jerome-pouiller · 2025-04-18T12:10:22Z

soc/silabs/silabs_siwx91x/Kconfig.defconfig

@@ -13,6 +13,21 @@ configdefault SYS_CLOCK_TICKS_PER_SEC
 	default 128 if !TICKLESS_KERNEL && SILABS_SLEEPTIMER_TIMER
 	default 1024 if SILABS_SLEEPTIMER_TIMER

+config SILABS_COMMANDER_SIGN
+	default n


n is already the default.

soc/silabs/silabs_siwx91x/Kconfig.defconfig

jerome-pouiller · 2025-04-18T12:13:14Z

scripts/west_commands/sign.py

+    parser = subparsers.add_parser('silabs_rps', help='Sign with Silicon Labs Commander')
+    parser.add_argument('--m4-ota-key', help='OTA key for signing (overrides Kconfig.defconfig default)')
+    parser.add_argument('--m4-private-key', help='Absolute path to private key file (overrides Kconfig.defconfig default)')
+    parser.add_argument('--commander-path', help='Absolute path to Commander executable (overrides Kconfig.defconfig default)')


These option should override CONFIG_M4_OTA_KEY, CONFIG_M4_PRIVATE_KEY and CONFIG_COMMANDER_PATH.

jerome-pouiller · 2025-04-18T12:21:02Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

+    )
+
+    # Add custom target to ensure signing is part of the build
+    add_custom_target(silabs_rps_sign_target


This is the user facing name. Don't we have a better name? signed-rps?

BTW, what is the purpose of this target? The user can already run west sign. Why do we provide west build -t silabs_rps_sign_target?

It would be more useful if the signing process was included in the default build.

Please, let the reviewer resolve the comments.

It would be more useful if the signing process was included in the default build.

soc/silabs/silabs_siwx91x/Kconfig.defconfig

jerome-pouiller · 2025-04-22T08:38:41Z

scripts/west_commands/sign.py

+   west sign -t silabs_commander -- --mic YOUR_OTA_KEY --encrypt YOUR_OTA_KEY --sign YOUR_PRIVATE_KEY.pem
+   or west sign -t silabs_commander
+
+For this to work, SILABS Commander tool must be installed in your PATH.


Silicon Labs (or possibly Silabs)

jerome-pouiller · 2025-04-22T08:41:57Z

scripts/west_commands/sign.py

+                        if line.strip().startswith('CONFIG_M4_OTA_KEY='):
+                            m4_ota_key = line.split('=')[1].strip().strip('"')
+                        elif line.strip().startswith('CONFIG_M4_PRIVATE_KEY='):
+                            m4_private_key = line.split('=')[1].strip().strip('"')


Do you need this at all? Can't you use:

build_conf.get('CONFIG_M4_OTA_KEY') build_conf.get('CONFIG_M4_PRIVATE_KEY')

No need , I added the fallback initially to cover potential retrieval issues. Now using .get() will resolve them.

jerome-pouiller · 2025-04-22T08:49:33Z

scripts/west_commands/sign.py

+                for path in default_paths:
+                    if os.path.isfile(path):
+                        commander_path = path
+                        break


KISS.

The system already manage that for you. (/usr/bin and /usr/bin/local are already part of the default PATH. If they are not, it is not your job to fix it).

jerome-pouiller · 2025-04-22T08:52:21Z

scripts/west_commands/sign.py

+        if not m4_private_key:
+            command.die("M4_PRIVATE_KEY not provided via --m4-private-key or set in prj.conf. Please specify --m4-private-key=/path/to/key or add CONFIG_M4_PRIVATE_KEY in prj.conf.")
+        if not commander_path:
+            command.die("Commander not found automatically. Please provide --commander-path=/path/to/commander or install it in your PATH.")


west sign already has the --tool-path option. Please use it:

def find_commandertool(cmd, args): if args.tool_path: commandertool = args.tool_path if not os.path.isfile(commandertool): cmd.die(f'--tool-path {commandertool}: no such file') else: commandertool = shutil.which('commander') if not commandertool: cmd.die('commander not found') return commandertool

I am not able to find find_commandertool in sign.py.

I meant, you can write a such function.

jerome-pouiller · 2025-04-22T08:57:18Z

scripts/west_commands/sign.py

+            command.die("ZEPHYR_BASE environment variable not set. Ensure you are running within a Zephyr build environment.")
+        workspace_base = pathlib.Path(zephyr_base)
+        if not workspace_base.is_dir():
+            command.die(f"Invalid ZEPHYR_BASE directory: {workspace_base}. Please check your Zephyr setup.")


I am not a specialist of the west commands, but I believe you should use zephyr_ext_common to retrieve ZEPHYR_BASE.

jerome-pouiller · 2025-04-22T09:05:49Z

scripts/west_commands/sign.py

+        kernel_name = build_conf.get('CONFIG_KERNEL_BIN_NAME', 'zephyr')
+
+        # Input and output files (using BUILD_DIR)
+        in_bin = b / 'zephyr' / f"{kernel_name}.bin.rps"  # Raw binary input


The name in_bin, the comment Raw binary input and the error messages No unsigned .bin found are confusing.
In fact, the input is not a binary file. It is a .rps file.

jerome-pouiller · 2025-04-22T09:10:14Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

 add_subdirectory(siwg917)
+zephyr_include_directories(.)
+zephyr_sources_ifdef(CONFIG_SOC_SERIES_SIWG917 soc.c)
+zephyr_sources_ifdef(CONFIG_WISECONNECT_NETWORK_STACK nwp_init.c)


Redundant with soc/silabs/silabs_siwx91x/siwg917/CMakeLists.txt. BTW, nwp_init.c has been replaced by nwp.c a few weeks ago.

jerome-pouiller · 2025-04-22T09:12:16Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

+
+# Custom signing with Silicon Labs Commander
+if(CONFIG_SiWX91X_SILABS_COMMANDER_SIGN)
+    # Define input and output files


Which input files? Outdated comments are worse than no comments.

jerome-pouiller · 2025-04-22T09:16:22Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

+# Custom signing with Silicon Labs Commander
+if(CONFIG_SiWX91X_SILABS_COMMANDER_SIGN)
+    # Define input and output files
+    set(SIGNED_RPS ${PROJECT_BINARY_DIR}/zephyr/${KERNEL_BIN_NAME}.bin.rps)


Do you need absolute path? I believe you need it only because you change WORKING_DIRECTORY below. You can probably remove remove all this complexity.

jerome-pouiller · 2025-04-22T09:18:18Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

+    )
+
+    # Add custom target to ensure signing is part of the build
+    add_custom_target(silabs_rps_sign_target


Please, let the reviewer resolve the comments.

It would be more useful if the signing process was included in the default build.

asmellby · 2025-04-30T09:02:33Z

scripts/west_commands/sign.py

@@ -23,6 +22,7 @@
 from runners.core import BuildConfiguration
 from zcmake import CMakeCache
 from zephyr_ext_common import Forceable, ZEPHYR_SCRIPTS
+from zephyr_ext_common import ZEPHYR_BASE


This import can be added to the above line instead of adding a new one.

asmellby · 2025-04-30T09:03:43Z

soc/silabs/silabs_siwx91x/Kconfig

+	help
+		Activates the signing process for the Zephyr binary using Silicon Labs Commander during the build process.
+
+config M4_OTA_KEY


These options should be SIWX91X_OTA_KEY and SIWX91X_PRIVATE_KEY in my opinion, in order to be properly namespaced.

asmellby · 2025-04-30T09:07:40Z

scripts/west_commands/sign.py

+
+        # Resolve paths
+        tool_path = pathlib.Path(commander_path)
+        if not tool_path.is_file():


This check seems to duplicate what's already been done in find_commandertool, so it should never happen. Consider making use of this detailed error message in the earlier check and removing it from here.

asmellby · 2025-04-30T09:08:18Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

 add_subdirectory(siwg917)
+zephyr_include_directories(.)


What is the purpose of changing the include path of the CMake build? This seems unrelated to signing support.

Aasimshaik11

I have resolved all the comments from @asmellby , @jerome-pouiller , @jhedberg , and @pdgendt . Could you mark these comments as resolved and provide any further suggestions or approval for my PR?

scripts/west_commands/sign.py

soc/silabs/silabs_siwx91x/Kconfig

Added changes to automate the signing of zephyr security using customtool during the west build process. Signed-off-by: Aasim Shaik <[email protected]>

Resolved request changes

pdgendt · 2025-05-05T09:28:57Z

Re-assigned to Silabs maintainers.

asmellby · 2025-05-07T10:03:34Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

@@ -12,3 +14,31 @@ set_property(GLOBAL APPEND PROPERTY extra_post_build_commands
    ${KERNEL_BIN_NAME}
    ${KERNEL_BIN_NAME}.rps
 )
+
+# Custom signing with Silicon Labs Commander
+if(CONFIG_SiWX91X_SILABS_COMMANDER_SIGN)


typo: SiWX91X -> SIWX91X (lowercase i)

jerome-pouiller · 2025-05-07T15:32:44Z

boards/silabs/radio_boards/siwx917_rb4338a/board.cmake

-
+# Set the flash file based on signing is enabled or not.
+set(FLASH_FILE ${PROJECT_BINARY_DIR}/zephyr_signed.bin.rps)
+if(NOT EXISTS ${FLASH_FILE})


Don't you think we should rather test CONFIG_SIWX91X_SILABS_COMMANDER_SIGN ?

jerome-pouiller · 2025-05-07T15:34:45Z

boards/silabs/radio_boards/siwx917_rb4338a/board.cmake

+# Set the flash file based on signing is enabled or not.
+set(FLASH_FILE ${PROJECT_BINARY_DIR}/zephyr_signed.bin.rps)
+if(NOT EXISTS ${FLASH_FILE})
+    set(FLASH_FILE ${PROJECT_BINARY_DIR}/zephyr.bin.rps)


Later in the script we refer to ${KERNEL_BIN_NAME}.rps or to ${CONFIG_KERNEL_BIN_NAME}_signed.bin.rps

jerome-pouiller · 2025-05-07T15:42:16Z

scripts/west_commands/sign.py

+
+To create a signed binary with the silabs_commander tool, run this from your build directory:
+
+   west sign -t silabs_commander -- --mic YOUR_OTA_KEY --encrypt YOUR_OTA_KEY --sign YOUR_PRIVATE_KEY.pem


I believe this does not match with get_security_configs().

jerome-pouiller · 2025-05-07T15:46:23Z

scripts/west_commands/sign.py

+            "--app", str(input_rps),
+            "--mic", siwx91x_ota_key,
+            "--encrypt", siwx91x_private_key,
+            "--sign", str(private_key_path)


Can you explain the difference between siwx91x_private_key and str(private_key_path)?

jerome-pouiller · 2025-05-07T15:47:09Z

scripts/west_commands/sign.py

+
+        private_key_path = pathlib.Path(siwx91x_private_key)
+        if not private_key_path.is_file():
+            command.die(f"Private key not found at {private_key_path}. Please ensure the path is correct.")


I suggest to group this check with the other related ones in get_security_configs().

jerome-pouiller · 2025-05-07T15:48:06Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

+    # Define the west sign command with explicit working directory and environment
+    set(WEST_SIGN_CMD
+        ${CMAKE_COMMAND} -E chdir ${CMAKE_BINARY_DIR}  # Explicitly set working directory
+        ${CMAKE_COMMAND} -E env "PATH=$ENV{PATH}:$ENV{WEST_ROOT}/bin" "ZEPHYR_WORKSPACE_ROOT=${CMAKE_CURRENT_SOURCE_DIR}"


I believe my previous comment is still relevant.

jerome-pouiller · 2025-05-07T15:53:34Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

+        ALL
+        DEPENDS ${KERNEL_BIN_NAME}.rps
+        COMMENT "Ensuring signing with Silicon Labs Commander is complete"
+    )


I believe this target is useless (the user can run west sign). However, you can add some entries to extra_post_build_commands to launch the signature automatically with west build.

jerome-pouiller · 2025-05-07T15:54:43Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

+    # Add custom command to invoke west sign during build
+    add_custom_command(
+        OUTPUT ${KERNEL_BIN_NAME}.rps
+        COMMAND ${CMAKE_COMMAND} -E echo "Forcing execution in ${PROJECT_BINARY_DIR}"


Is this trace relevant? I believe it bring any useful information to the user.

jerome-pouiller · 2025-05-07T15:56:41Z

soc/silabs/silabs_siwx91x/Kconfig

+config SIWX91X_PRIVATE_KEY
+	string "Private Key Path for Signing (sourced from prj.conf)"
+	help
+	  Path to private key file for signing with Silicon Labs Commander, from prj.conf, relative to board directory.


These options should not be displayed to the user if SOC_FAMILY_SILABS_SIWX91X is not selected. You need to if SOC_FAMILY_SILABS_SIWX91X.

Aasimshaik11 force-pushed the zephyr-boot-security branch 2 times, most recently from 7541fb9 to 7052317 Compare March 26, 2025 11:28

Aasimshaik11 force-pushed the zephyr-boot-security branch from 7052317 to da52b68 Compare April 4, 2025 06:46

jerome-pouiller self-requested a review April 4, 2025 09:55

asmellby requested changes Apr 4, 2025

View reviewed changes

Aasimshaik11 force-pushed the zephyr-boot-security branch 4 times, most recently from 726af6b to b9ddd52 Compare April 8, 2025 10:31

Aasimshaik11 marked this pull request as ready for review April 9, 2025 04:34

github-actions bot added platform: Silabs Silicon Labs area: West West utility labels Apr 9, 2025

github-actions bot requested review from carlescufi, jhedberg, Martinhoff-maker, pdgendt and swinslow April 9, 2025 04:35

github-actions bot assigned pdgendt Apr 9, 2025

pdgendt previously requested changes Apr 9, 2025

View reviewed changes

Aasimshaik11 force-pushed the zephyr-boot-security branch 2 times, most recently from 6015f25 to d329322 Compare April 9, 2025 09:35

Aasimshaik11 force-pushed the zephyr-boot-security branch 2 times, most recently from 82ac254 to 2a2c05b Compare April 9, 2025 11:29

jhedberg reviewed Apr 9, 2025

View reviewed changes

soc/silabs/silabs_siwx91x/Kconfig.soc Outdated Show resolved Hide resolved

soc/silabs/silabs_siwx91x/Kconfig.soc Outdated Show resolved Hide resolved

jerome-pouiller reviewed Apr 9, 2025

View reviewed changes

jerome-pouiller requested changes Apr 9, 2025

View reviewed changes

Aasimshaik11 force-pushed the zephyr-boot-security branch 3 times, most recently from 46bdfdf to c6cbbf6 Compare April 18, 2025 06:20

jerome-pouiller reviewed Apr 18, 2025

View reviewed changes

soc/silabs/silabs_siwx91x/Kconfig.defconfig Outdated Show resolved Hide resolved

Aasimshaik11 force-pushed the zephyr-boot-security branch from c6cbbf6 to f9c082c Compare April 20, 2025 08:14

github-actions bot added the area: Samples Samples label Apr 20, 2025

github-actions bot requested review from kartben and nashif April 20, 2025 08:14

Aasimshaik11 force-pushed the zephyr-boot-security branch 3 times, most recently from 2ac309f to 009ada8 Compare April 21, 2025 05:42

jerome-pouiller requested changes Apr 22, 2025

View reviewed changes

Aasimshaik11 force-pushed the zephyr-boot-security branch 3 times, most recently from 964683e to 82595cd Compare April 25, 2025 05:16

asmellby reviewed Apr 30, 2025

View reviewed changes

Aasimshaik11 force-pushed the zephyr-boot-security branch from 82595cd to 7107751 Compare April 30, 2025 10:52

Aasimshaik11 commented May 5, 2025

View reviewed changes

pdgendt reviewed May 5, 2025

View reviewed changes

scripts/west_commands/sign.py Outdated Show resolved Hide resolved

soc/silabs/silabs_siwx91x/Kconfig Outdated Show resolved Hide resolved

Scripts: Security enablement during west build

69b832a

Added changes to automate the signing of zephyr security using customtool during the west build process. Signed-off-by: Aasim Shaik <[email protected]>

Aasimshaik11 force-pushed the zephyr-boot-security branch from 7107751 to 69b832a Compare May 5, 2025 08:03

pdgendt assigned jhedberg and asmellby and unassigned pdgendt May 5, 2025

asmellby reviewed May 7, 2025

View reviewed changes

jerome-pouiller reviewed May 7, 2025

View reviewed changes


		To create a signed binary with the silabs_commander tool, run this from your build directory:

		west sign -t silabs_commander -- --mic YOUR_OTA_KEY --encrypt YOUR_OTA_KEY --sign YOUR_PRIVATE_KEY.pem

Scripts: Security enablement during west build #87669

Are you sure you want to change the base?

Scripts: Security enablement during west build #87669

Conversation

Aasimshaik11 commented Mar 26, 2025

github-actions bot commented Mar 26, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aasimshaik11 Apr 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdgendt commented Apr 9, 2025

jerome-pouiller Apr 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerome-pouiller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aasimshaik11 Apr 7, 2025 •

edited

Loading

jerome-pouiller Apr 9, 2025 •

edited

Loading